fix: document ESM resolution behavior for validator and similar packages (#189) by yunus25jmi1 · Pull Request #193 · cloudflare/vinext

yunus25jmi1 · 2026-02-28T08:40:58Z

Summary

Documents how vinext's existing noExternal: true setting resolves ESM resolution issues for packages like validator (fixes #189).

Problem

Users importing certain ESM packages (like validator) encountered Cannot find module errors in dev server:

Cannot find module '...\validator\es\lib\util\assertString'
imported from '...\validator\es\lib\isEmail.js'

How It's Already Fixed

vinext sets noExternal: true in all server environments (RSC and SSR), which forces all dependencies through Vite's transform pipeline. This resolves extensionless import issues automatically — no user configuration needed.

Changes

Documentation: Added troubleshooting section explaining the existing behavior
Test fixture: Added tests/fixtures/ecosystem/validator/ to verify validator works out of the box
Ecosystem test: Added test case confirming validator SSR imports work
Comments: Added explanatory comments in index.ts referencing Raise Cannot find module after access local dev server web page. #189

southpolesteve · 2026-03-03T23:20:00Z

/bigbonk review please

ask-bonk

Review Summary

This PR adds useful functionality: auto-detecting problematic ESM packages and propagating ssr.noExternal to the RSC environment, which fixes a real issue (#189). The test coverage is thorough and the core approach (merging user config with auto-detected packages, propagating to both rsc and ssr environments) is sound.

However, there are several issues that need to be addressed:

Must Fix

package-lock.json must not be committed — this is a pnpm monorepo. An npm lockfile is incorrect.
Wrong GitHub URL in code comment — references nicepkg/vinext instead of cloudflare/vinext.
Unrelated changes bundled in — The Space_Grotesk export (issue #190) and its test should be in a separate PR. This PR claims to fix #189 only.

Should Fix

Questionable package list — cuid and nanoid don't have the described ESM issue. uuid v9+ ships proper ESM. Only validator has a verified reproduction.
RegExp items silently dropped — Vite's resolve.noExternal supports RegExp; the comment claiming otherwise is inaccurate.

Minor

Fixture uses wrong Vite version — ^6.0.0 instead of the project's ^7.3.1.
Documentation references unverified version (v0.0.16+).

ask-bonk · 2026-03-03T23:23:09Z

package-lock.json

@@ -0,0 +1,5958 @@
+{


Must remove. This is a pnpm monorepo (uses pnpm-lock.yaml). A 5,958-line npm package-lock.json should not be committed — it's the wrong package manager's lockfile and will cause confusion.

ask-bonk · 2026-03-03T23:23:09Z

packages/vinext/src/index.ts

+ * Adding them to `resolve.noExternal` forces Vite to bundle them through
+ * its transform pipeline, which resolves extensionless imports correctly.
+ *
+ * See: https://github.com/nicepkg/vinext/issues/189


Wrong repository URL. The repo is cloudflare/vinext, not nicepkg/vinext.

Suggested change

* See: https://github.com/nicepkg/vinext/issues/189

* See: https://github.com/cloudflare/vinext/issues/189

ask-bonk · 2026-03-03T23:23:09Z

packages/vinext/src/index.ts

+  "validator",     // validator/es/lib/isEmail.js can't resolve ./util/assertString
+  "uuid",          // Similar ESM resolution issues
+  "date-fns",      // date-fns/esm has complex internal structure
+  "cuid",


cuid and nanoid do not have the ESM resolution issue described in the JSDoc comment above. Both ship clean ESM with proper exports maps. Adding packages speculatively to this allowlist forces Vite to bundle them through its transform pipeline unnecessarily, which:

Increases dev server startup time

Can break packages that rely on being externalized

Erodes trust in the list (users will question every entry)

uuid v9+ also ships proper ESM with file extensions. The comment "Similar ESM resolution issues" is unverified.

Recommendation: Only include packages with verified reproduction cases. Keep validator (verified in #189) and date-fns (known complex ESM internal structure). Remove cuid, nanoid, and verify uuid before including it.

ask-bonk · 2026-03-03T23:23:09Z

packages/vinext/src/index.ts

+  } else if (Array.isArray(userSsrNoExternal)) {
+    for (const item of userSsrNoExternal) {
+      if (typeof item === "string") packages.add(item);
+      // RegExp items are skipped — can't propagate to resolve.noExternal


The comment says RegExp items "can't propagate to resolve.noExternal" — this is inaccurate. Vite's resolve.noExternal does support RegExp values (it accepts string | RegExp | (string | RegExp)[] | true). If you're choosing to skip them for implementation simplicity, the comment should reflect that, e.g. "RegExp items are not propagated — only string package names are supported".

ask-bonk · 2026-03-03T23:23:09Z

packages/vinext/src/shims/font-google.ts

 export const JetBrains_Mono = createFontLoader("JetBrains Mono");
 export const Geist = createFontLoader("Geist");
 export const Geist_Mono = createFontLoader("Geist Mono");
+export const Space_Grotesk = createFontLoader("Space Grotesk");


This change (adding Space_Grotesk export) is for issue #190, not #189. It should be in a separate PR to keep the diff focused and reviewable. The PR title and description only mention #189.

ask-bonk · 2026-03-03T23:23:09Z

tests/fixtures/ecosystem/validator/package.json

+  },
+  "devDependencies": {
+    "vinext": "workspace:*",
+    "vite": "^6.0.0"


The project uses Vite 7 ("vite": "^7.3.1" in root package.json). This fixture should match the workspace version.

Suggested change

"vite": "^6.0.0"

"vite": "^7.3.1"

ask-bonk · 2026-03-03T23:23:09Z

.agents/skills/migrate-to-vinext/references/troubleshooting.md

+```
+
+**Cause:** Some ESM packages have complex internal import structures that Node.js module resolution can't handle when externalized. By default, Vite externalizes dependencies in SSR (loads them via Node.js) instead of transforming them through Vite's pipeline.
+


Where does v0.0.16+ come from? This version number should match the actual release version where this feature will ship. If uncertain, remove the version reference to avoid documenting an inaccurate version number.

ask-bonk · 2026-03-03T23:23:09Z

tests/fixtures/ecosystem/validator/vite.config.ts

+
+export default defineConfig({
+  plugins: [vinext()],
+  // No manual ssr.noExternal needed - vinext auto-detects validator!


"auto-detects" is somewhat misleading — it's a hardcoded allowlist check, not runtime detection of ESM issues. Consider:

Suggested change

// No manual ssr.noExternal needed - vinext auto-detects validator!

// No manual ssr.noExternal needed - vinext includes validator in its known problematic ESM packages list

ask-bonk · 2026-03-03T23:23:20Z

The review was submitted successfully. Here's a summary of what I flagged:

Review Posted on PR #193

Verdict: Changes Requested

Must Fix

package-lock.json (5,958 lines) — Must be removed. This is a pnpm monorepo; an npm lockfile is incorrect.
Wrong GitHub URL — Comment at index.ts:558 references nicepkg/vinext instead of cloudflare/vinext.
Unrelated changes — The Space_Grotesk export (issue Some google fonts may have issues [vite] Internal server error: (0 , __vite_ssr_import_2__.Space_Grotesk) is not a function #190) and its test in font-google.ts/font-google.test.ts should be a separate PR.

Should Fix

Speculative package list — cuid and nanoid in KNOWN_PROBLEMATIC_ESM_PACKAGES don't have the described ESM issue. uuid v9+ ships proper ESM. Only validator has a verified reproduction from the issue.
Inaccurate comment about RegExp — The code comment claims RegExp "can't propagate to resolve.noExternal", but Vite's API does support RegExp there.

Minor

Wrong Vite version in test fixture — uses ^6.0.0 instead of the project's ^7.3.1.
Unverified version reference (v0.0.16+) in troubleshooting docs.
Misleading "auto-detects" comment in fixture config — it's a hardcoded allowlist, not runtime detection.

github run

yunus25jmi1 · 2026-03-04T01:15:18Z

@elithrar @southpolesteve kindly review the changes.

yunus25jmi1 · 2026-03-05T00:33:55Z

kindly review the changes.

pkg-pr-new · 2026-03-05T00:53:27Z

Open in StackBlitz

npm i https://pkg.pr.new/vinext@193

commit: 8bb5c1c

yunus25jmi1 · 2026-03-05T01:08:53Z

@southpolesteve The workflow is all green.

james-elicx · 2026-03-08T09:54:07Z

/bigbonk re-review this pr please

ask-bonk

Review

This PR has been significantly cleaned up since the previous review — the problematic package-lock.json, Space_Grotesk changes, speculative package allowlist, and wrong GitHub URL are all gone. What remains is a focused, minimal PR: 6 lines of explanatory comments in index.ts, a troubleshooting doc section, and a validator ecosystem test fixture.

All CI checks pass. The changes are sound.

I have a few minor notes below, but nothing blocking.

ask-bonk · 2026-03-08T09:57:29Z

.agents/skills/migrate-to-vinext/references/troubleshooting.md

+
+**Cause:** Some ESM packages have complex internal import structures that Node.js module resolution can't handle when externalized.
+
+**vinext Fix:** vinext sets `noExternal: true` in all server environments (RSC and SSR), which forces all dependencies through Vite's transform pipeline. This resolves extensionless import issues automatically.


Minor: the PR title and description still say "document ESM resolution workaround" and describe adding packages to ssr.noExternal manually, but the actual doc here correctly says "No configuration needed — this is the default behavior." The PR description is stale/misleading relative to what the code actually does. Worth updating the PR description to match the actual content (documentation of existing behavior, not a workaround).

ask-bonk · 2026-03-08T09:57:29Z

tests/ecosystem.test.ts

+  let fetchPage: (pathname: string) => Promise<{ html: string; status: number }>;
+
+  beforeAll(async () => {
+    const fixture = await startFixture("validator", 5011);


Nit: the existing fixtures use sequential ports 4400–4404. Port 5011 is a noticeable jump. Consider using 4405 to keep the sequence consistent and make it easier to spot conflicts.

Suggested change

const fixture = await startFixture("validator", 5011);

const fixture = await startFixture("validator", 4405);

ask-bonk · 2026-03-08T09:57:29Z

tests/build-optimization.test.ts

    expect(result).not.toContain("a;b");
  });
 });
+


This file's only change is adding a trailing newline. Harmless, but appears to be an accidental edit — the file has nothing to do with the validator ESM fix. Consider dropping it from the PR to keep the diff focused.

ask-bonk · 2026-03-08T09:57:29Z

packages/vinext/src/index.ts

          // Skip when targeting bundled runtimes (Cloudflare/Nitro bundle everything).
+          // Also propagate noExternal for problematic ESM packages so they
+          // are bundled by Vite instead of loaded natively (where Node's
+          // strict ESM resolution rejects extensionless imports).


Nit: this comment is slightly misleading in context. The noExternal: true on line 2068 forces all dependencies through Vite, not just "problematic ESM packages." The existing comment block (lines 2053–2061) already explains the rationale clearly. This addendum reads like there's a separate per-package mechanism, but there isn't — it's the same blanket noExternal: true.

Consider trimming to something like:

// This also resolves extensionless-import issues in packages like // `validator` (see #189) by routing them through Vite's resolver.

ask-bonk · 2026-03-08T09:57:29Z

packages/vinext/src/index.ts


+          // For App Router, propagate noExternal to BOTH rsc and ssr
+          // environments (the top-level ssr.noExternal only affects the
+          // default ssr env, not the rsc env).


Same note as above — the comment says "propagate noExternal" as if this is a new behavior being added. In reality, lines 2154 and 2177 already had noExternal: true before this PR. The comment is purely additive documentation of existing behavior, which is fine, but the phrasing could be clearer that this is explaining why the existing code works, not describing a change.

Minor — no action required.

ask-bonk · 2026-03-08T09:57:38Z

Review posted successfully. Here's a summary of what I flagged:

Review Summary for PR #193

Verdict: Looks good overall — all CI passes, the PR is well-scoped after cleanup.

The PR adds:

6 lines of explanatory comments in index.ts (no behavioral changes)
A troubleshooting doc section explaining that noExternal: true handles ESM resolution automatically
A validator ecosystem test fixture proving it works out of the box

Minor notes posted:

Stale PR description — still describes a manual ssr.noExternal workaround, but the actual content documents that no configuration is needed
Port number jump — validator fixture uses port 5011 instead of continuing the 4400–4404 sequence (suggested 4405)
Unrelated trailing newline in build-optimization.test.ts — should be dropped from the diff
Comment phrasing in index.ts — reads like a new per-package mechanism is being added, when it's really just documenting existing noExternal: true behavior
Second comment block — same minor phrasing note, no action required

github run

cloudflare#189) Vinext now automatically detects installed packages with broken ESM resolution and adds them to resolve.noExternal for both the rsc and ssr environments. This fixes 'Cannot find module' errors when using packages like validator that have extensionless internal ESM imports. Root cause: Vite externalizes dependencies for SSR by default, but some packages use extensionless imports (e.g. ./util/assertString) that fail under Node.js strict ESM resolution. Additionally, Vite 7's Environment API means ssr.noExternal only affects the ssr environment, not the rsc environment where server components run. Changes: - Add KNOWN_PROBLEMATIC_ESM_PACKAGES list (validator, date-fns) - Add collectNoExternalPackages() to merge user config with auto-detected packages - Propagate noExternal to both rsc and ssr environments - Add validator ecosystem test fixture and integration tests - Add troubleshooting documentation Fixes cloudflare#189

yunus25jmi1 · 2026-03-08T10:09:42Z

@james-elicx changes pushed. re-review the changes.

yunus25jmi1 force-pushed the fix/issue-189-validator-esm-resolution branch from acf79f1 to 7f935ff Compare February 28, 2026 09:09

ask-bonk bot requested changes Mar 3, 2026

View reviewed changes

yunus25jmi1 force-pushed the fix/issue-189-validator-esm-resolution branch from 02b9d86 to 69cdd65 Compare March 4, 2026 00:56

yunus25jmi1 force-pushed the fix/issue-189-validator-esm-resolution branch 2 times, most recently from 1d83370 to b7b5402 Compare March 5, 2026 00:52

yunus25jmi1 force-pushed the fix/issue-189-validator-esm-resolution branch from b7b5402 to e476e70 Compare March 5, 2026 01:05

yunus25jmi1 force-pushed the fix/issue-189-validator-esm-resolution branch 2 times, most recently from 6b707a7 to cfb7c8a Compare March 8, 2026 09:50

ask-bonk bot reviewed Mar 8, 2026

View reviewed changes

yunus25jmi1 changed the title ~~fix: document ESM resolution workaround for validator and similar packages (#189)~~ fix: document ESM resolution behavior for validator and similar packages (#189) Mar 8, 2026

yunus25jmi1 force-pushed the fix/issue-189-validator-esm-resolution branch from cfb7c8a to 75a8d4e Compare March 8, 2026 10:04

	* See: https://github.com/nicepkg/vinext/issues/189
	* See: https://github.com/cloudflare/vinext/issues/189

		```

		Cause: Some ESM packages have complex internal import structures that Node.js module resolution can't handle when externalized. By default, Vite externalizes dependencies in SSR (loads them via Node.js) instead of transforming them through Vite's pipeline.

	// No manual ssr.noExternal needed - vinext auto-detects validator!
	// No manual ssr.noExternal needed - vinext includes validator in its known problematic ESM packages list


		Cause: Some ESM packages have complex internal import structures that Node.js module resolution can't handle when externalized.

		vinext Fix: vinext sets `noExternal: true` in all server environments (RSC and SSR), which forces all dependencies through Vite's transform pipeline. This resolves extensionless import issues automatically.

	const fixture = await startFixture("validator", 5011);
	const fixture = await startFixture("validator", 4405);

Conversation

yunus25jmi1 commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

How It's Already Fixed

Changes

Uh oh!

southpolesteve commented Mar 3, 2026

Uh oh!

ask-bonk bot left a comment

Choose a reason for hiding this comment

Review Summary

Must Fix

Should Fix

Minor

Uh oh!

ask-bonk bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot commented Mar 3, 2026

Review Posted on PR #193

Must Fix

Should Fix

Minor

Uh oh!

yunus25jmi1 commented Mar 4, 2026

Uh oh!

yunus25jmi1 commented Mar 5, 2026

Uh oh!

pkg-pr-new bot commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yunus25jmi1 commented Mar 5, 2026

Uh oh!

james-elicx commented Mar 8, 2026

Uh oh!

ask-bonk bot left a comment

Choose a reason for hiding this comment

Review

Uh oh!

ask-bonk bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot commented Mar 8, 2026

Review Summary for PR #193

Uh oh!

yunus25jmi1 commented Feb 28, 2026 •

edited

Loading

pkg-pr-new bot commented Mar 5, 2026 •

edited

Loading